From: Daniel van Vugt <daniel.van.vugt@canonical.com>
Date: Fri, 14 Feb 2020 10:24:58 +0000 (+0800)
Subject: iconGrid: Create icon clones in a separate loop
X-Git-Tag: archive/raspbian/50.5-1+rpi1~1^2~772^2^2~96
X-Git-Url: https://dgit.raspbian.org/%22http://www.example.com/cgi/%22/%22http:/www.example.com/cgi/%22?a=commitdiff_plain;h=e781e1fdba9faf6a02f8cca33e308070620266eb;p=gnome-shell.git

iconGrid: Create icon clones in a separate loop

For reasons not yet fully understood, `Main.uiGroup.add_actor` takes around
10 milliseconds to complete.

Because of this, each `actor.opacity = 0` has a good chance of falling
on a different frame. And when it does, `_opacityChangedId` also lands
on multiple different frames each incurring a separate relayout cycle.
It is this excessive number of relayouts that causes stuttering in the
icon grid animation (#2065). But it is the slowness of `uiGroup.add_actor`
that causes the number to be excessive when it should be one.

By creating the clones and adding them to `uiGroup` early, we then enable
the existing loop starting the animation to complete within a single frame.
And by completing within a single frame all the opacity changes land within
the same frame interval, thus incurring only a single relayout instead of
many.

This issue went unnoticed until 004a5e1042 (!704), after which the slow
emissions of `notify::opacity` became a more visible performance problem.

Closes: https://gitlab.gnome.org/GNOME/gnome-shell/issues/2065

https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/1002
---

diff --git a/js/ui/iconGrid.js b/js/ui/iconGrid.js
index 19d019f4d..25cd14099 100644
--- a/js/ui/iconGrid.js
+++ b/js/ui/iconGrid.js
@@ -556,15 +556,25 @@ var IconGrid = GObject.registerClass({
         }, Infinity);
         let normalization = maxDist - minDist;
 
-        for (let index = 0; index < actors.length; index++) {
-            let actor = actors[index];
+        actors.forEach(actor => {
+            let clone = new Clutter.Clone({ source: actor });
+            this._clonesAnimating.push(clone);
+            Main.uiGroup.add_actor(clone);
+        });
+
+        /*
+         * ^
+         * | These need to be separate loops because Main.uiGroup.add_actor
+         * | is excessively slow if done inside the below loop and we want the
+         * | below loop to complete within one frame interval (#2065, !1002).
+         * v
+         */
+
+        this._clonesAnimating.forEach(actorClone => {
+            let actor = actorClone.source;
             actor.opacity = 0;
             actor.reactive = false;
 
-            let actorClone = new Clutter.Clone({ source: actor });
-            this._clonesAnimating.push(actorClone);
-            Main.uiGroup.add_actor(actorClone);
-
             let [width, height] = this._getAllocatedChildSizeAndSpacing(actor);
             actorClone.set_size(width, height);
             let scaleX = sourceScaledWidth / width;
@@ -631,7 +641,7 @@ var IconGrid = GObject.registerClass({
 
             actorClone.ease(movementParams);
             actorClone.ease(fadeParams);
-        }
+        });
     }
 
     _getAllocatedChildSizeAndSpacing(child) {